Skip to content

Fix infinite loop in replaceStringInfoString#1750

Merged
tuhaihe merged 2 commits into
mainfrom
string_replace_fix
Jul 14, 2026
Merged

Fix infinite loop in replaceStringInfoString#1750
tuhaihe merged 2 commits into
mainfrom
string_replace_fix

Conversation

@Smyatkin-Maxim

@Smyatkin-Maxim Smyatkin-Maxim commented May 16, 2026

Copy link
Copy Markdown
Contributor

The function had a few problems:

  • replace ="a" and replacement="ab" would give an infinite loop leading to OOM. We have a similar case in pg_dump: replace="range", replacement="multirange"
  • Copying the whole string each time there is a match
  • Empty replace pattern also leads to infinite loop

We can still do better: for example count number of matches and do a single allocation, or even run the replacement inplace. But it's probably not worth it.

What does this PR do?

Type of Change

  • Bug fix (non-breaking change)
  • New feature (non-breaking change)
  • Breaking change (fix or feature with breaking changes)
  • Documentation update

Breaking Changes

Test Plan

  • Unit tests added/updated
  • Integration tests added/updated
  • Passed make installcheck
  • Passed make -C src/test installcheck-cbdb-parallel
  • gpupgrade of regression database

Impact

Performance:

User-facing changes:

Dependencies:

Checklist

Additional Context

CI Skip Instructions


@leborchuk leborchuk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no replace String Info in https://github.com/postgres/postgres/blob/master/src/common/stringinfo.c so we need to implement our own solution

@yjhjstz yjhjstz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yjhjstz yjhjstz force-pushed the string_replace_fix branch from 4dc5dd0 to 28a4066 Compare May 18, 2026 18:23
Comment thread src/common/stringinfo.c Outdated
Comment thread src/common/stringinfo.c Outdated
Smyatkin-Maxim and others added 2 commits July 14, 2026 11:34
The function had a few problems:
- replace ="a" and replacement="ab" would give an infinite loop leading
  to OOM. We have a similar case in pg_dump: replace="range",
  replacement="multirange"
- Copying the whole string each time there is a match
- Empty replace pattern also leads to infinite loop

We can still do better: for example count number of matches and do a
single allocation, or even run the replacement inplace. But it's
probably not worth it.
@tuhaihe tuhaihe force-pushed the string_replace_fix branch from 7c57076 to 39bb89c Compare July 14, 2026 03:34
@tuhaihe tuhaihe merged commit 7f5c362 into main Jul 14, 2026
45 checks passed
@tuhaihe tuhaihe deleted the string_replace_fix branch July 14, 2026 05:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants